Skip to content

feat: implement XML attachment file naming functionality (backport #253)#257

Open
mergify[bot] wants to merge 2 commits into
version-16-hotfixfrom
mergify/bp/version-16-hotfix/pr-253
Open

feat: implement XML attachment file naming functionality (backport #253)#257
mergify[bot] wants to merge 2 commits into
version-16-hotfixfrom
mergify/bp/version-16-hotfix/pr-253

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented May 12, 2026

  • Added E Invoice Settings field for global autonaming pattern for XML attachements.
  • Added get_xml_attachment_file_base_name function to resolve file names.
  • Introduced unit tests for various naming scenarios in test_xml_attachment_naming.py.
  • Updated Sales Invoice logic to utilize the new naming function for XML file attachments.
  • Integrated autonaming pattern into "Download XML" button on Sales Invoice.

Ref: IBT-164
closes #245


This is an automatic backport of pull request #253 done by Mergify.

@mergify mergify Bot added the conflicts label May 12, 2026
@mergify
Copy link
Copy Markdown
Contributor Author

mergify Bot commented May 12, 2026

Cherry-pick of d45a1f0 has failed:

On branch mergify/bp/version-16-hotfix/pr-253
Your branch is up to date with 'origin/version-16-hotfix'.

You are currently cherry-picking commit d45a1f0.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   eu_einvoice/european_e_invoice/custom/sales_invoice.py
	new file:   eu_einvoice/european_e_invoice/custom/test_sales_invoice.py
	modified:   eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.json
	both modified:   eu_einvoice/locale/de.po
	both modified:   eu_einvoice/locale/main.pot

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 12, 2026

Greptile Summary

This backport of #253 adds configurable XML attachment file naming to the E Invoice workflow: a new auto_name_format_for_xml_file field in E Invoice Settings lets users define a naming pattern (using Frappe's naming series syntax) that replaces the hard-coded {doc.name}.xml default. However, the automated backport left unresolved git merge conflicts in three files that must be fixed before merging.

  • sales_invoice.py: New get_xml_attachment_file_base_name function resolves the naming pattern via parse_naming_series, sanitizes the result with get_safe_file_name, and falls back to doc.name on failure.
  • e_invoice_settings.json: Contains two unresolved conflict blocks making it invalid JSON — the doctype cannot be loaded.
  • de.po / main.pot: Both locale files have multiple unresolved conflict markers that corrupt the gettext format.

Confidence Score: 2/5

Not safe to merge — three files contain raw git conflict markers that make the doctype JSON invalid and break the German locale.

The Python implementation in sales_invoice.py and the new tests are well-constructed, but the automated backport left e_invoice_settings.json as unparseable JSON (two unresolved conflict blocks) and both locale files corrupted. As long as those conflict markers remain, importing the app or running bench migrate will fail on the doctype definition, and translation loading will error for the de locale.

e_invoice_settings.json, eu_einvoice/locale/de.po, and eu_einvoice/locale/main.pot all require manual conflict resolution before this branch can be used.

Important Files Changed

Filename Overview
eu_einvoice/european_e_invoice/custom/sales_invoice.py Adds get_xml_attachment_file_base_name and _no_series_counter; updates download_xrechnung to pass the full doc and use the new naming logic. Python logic is sound.
eu_einvoice/european_e_invoice/custom/test_sales_invoice.py New unit tests covering fallback, field interpolation, hash stripping, slash sanitization, and pattern resolution — comprehensive coverage of the new naming function.
eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.json Contains unresolved git merge conflict markers at two locations, making the file invalid JSON. The doctype cannot be loaded in this state.
eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.py Adds the auto_name_format_for_xml_file typed field annotation — minimal, correct change.
eu_einvoice/locale/de.po Contains unresolved merge conflict markers at multiple locations; the gettext parser will reject this file, breaking German translations.
eu_einvoice/locale/main.pot Contains unresolved merge conflict markers at multiple locations; invalid .pot file that will break translation tooling.

Comments Outside Diff (1)

  1. eu_einvoice/locale/de.po, line 234-238 (link)

    P0 Unresolved merge conflict markers in gettext file

    de.po and main.pot both contain unresolved <<<<<<< HEAD / ======= / >>>>>>> d45a1f0 conflict markers throughout. The gettext parser will reject these files outright, breaking translation loading for the de locale. All conflicts in both locale files must be resolved — keeping the newer line-number references and adding the new translation strings — before merging.

    Fix in Cursor

Fix All in Cursor

Reviews (1): Last reviewed commit: "feat: implement XML attachment file nami..." | Re-trigger Greptile

Comment thread eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.json Outdated
@dafrose dafrose force-pushed the mergify/bp/version-16-hotfix/pr-253 branch from 0e23163 to b40013c Compare May 15, 2026 13:38
@dafrose dafrose requested a review from barredterra May 15, 2026 13:50
dafrose and others added 2 commits May 15, 2026 17:03
* feat(eu_einvoice): configurable base name for auto-attached XRechnung XML

Add optional pattern on E Invoice Settings (auto_name_format_for_xml_file) for the
file base name when auto-attaching XML on Sales Invoice submit. Empty pattern keeps
using the document name; otherwise use naming-series-style segments (dot-separated,
parse_naming_series, {field} placeholders, date tokens), strip leading # per
segment so counter-only parts do not consume Series, sanitize with get_safe_file_name,
and fall back to the document name on error.

Colocate get_xml_attachment_file_base_name with Sales Invoice custom logic and cover
naming edge cases in test_sales_invoice.py.

Refresh E Invoice Settings controller and de / template catalogs for the new strings.

* refactor(eu_einvoice): enhance download_xrechnung to use configurable base name

Update the download_xrechnung function to retrieve the Sales Invoice document
and check permissions before generating the XML file.
The filename now utilizes a configurable base name from E Invoice Settings,
improving flexibility for file naming.
This change ensures that the generated XML file adheres to the specified naming format.

* chore(eu_einvoice): update E Invoice Settings and localization files

Rearrange fields in e_invoice_settings.json for better organization, adding a label for
the XML Settings section. Update localization files (de.po and main.pot) to reflect
changes in field names and add new translations for XML Settings.
Adjust POT creation dates for consistency.

* refactor(eu_einvoice): centralize sales invoice xml stem naming

Resolve the downloadable XML filename stem in get_xml_attachment_file_base_name:
read *Auto name format for XML file* from **E Invoice Settings** when the pattern
argument is omitted, accept an optional keyword-only pattern override, and build
the stem with parse_naming_series plus a no-op number generator so naming has no
series counter DB side effects. Sanitize with get_safe_file_name and fall back to
doc.name when the pattern is empty or fails.

Call sites use the helper without duplicating settings reads. Tests pass pattern=
for empty or whitespace patterns, and assert the settings-backed path by patching
frappe.get_single_value only for **E Invoice Settings** / auto_name_format_for_xml_file.

---------

Co-authored-by: Daniel Rose <26166128+dafrose@users.noreply.github.com>
(cherry picked from commit d45a1f0)
Co-authored-by: Cursor <cursoragent@cursor.com>
Frappe v15 does not expose get_safe_file_name on file utils; mirror the v17+
implementation beside get_xml_attachment_file_base_name as _get_safe_file_name.
@dafrose dafrose force-pushed the mergify/bp/version-16-hotfix/pr-253 branch from b40013c to f8aa2b0 Compare May 15, 2026 15:04
@dafrose dafrose removed the conflicts label Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant